[googleapis_auth] Support external_account credentials for Workload Identity Federation#727
[googleapis_auth] Support external_account credentials for Workload Identity Federation#727kevmoo merged 1 commit intofeature_ADCfrom
Conversation
…dentity Federation Features: - Add support for `external_account` type in Application Default Credentials (ADC). - Add `StsAuthClient` and `clientViaStsTokenExchange` to handle token exchange with Google SecurityToken Service. - Implement file-sourced and url-sourced credentials for STS exchange. Tests: - Add `sts_auth_client_test.dart` to verify token exchange functionality. - Update `adc_test.dart` to verify `external_account` flow.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the authentication capabilities of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
kevmoo
changed the title
PR HealthBreaking changes ✔️
This check can be disabled by tagging the PR with Unused Dependencies ✔️
For details on how to fix these, see dependency_validator. This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with License Headers ✔️
All source files should start with a license header. This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for external_account credentials for Workload Identity Federation, which is a valuable addition. The implementation introduces a new StsAuthClient to handle the STS token exchange and integrates it into the Application Default Credentials flow. The changes are well-tested. I have one suggestion to improve the robustness of token retrieval from the credential source.
| Future<String> _getSubjectToken() async { | ||
| if (_credentialSource.containsKey('file')) { | ||
| final fileField = _credentialSource['file'] as String; | ||
| return await File(fileField).readAsString(); | ||
| } else if (_credentialSource.containsKey('url')) { | ||
| final url = _credentialSource['url'] as String; | ||
| final headers = _credentialSource['headers'] as Map<String, dynamic>?; | ||
| final format = _credentialSource['format'] as Map<String, dynamic>?; | ||
|
|
||
| final parsedHeaders = headers?.map( | ||
| (key, value) => MapEntry(key, value.toString()), | ||
| ); | ||
|
|
||
| final response = await baseClient.get( | ||
| Uri.parse(url), | ||
| headers: parsedHeaders, | ||
| ); | ||
|
|
||
| if (response.statusCode != 200) { | ||
| throw Exception( | ||
| 'Failed to retrieve subject token from URL: $url. ' | ||
| 'Status code: ${response.statusCode}, Body: ${response.body}', | ||
| ); | ||
| } | ||
|
|
||
| var token = response.body; | ||
|
|
||
| if (format != null && format['type'] == 'json') { | ||
| final json = jsonDecode(token) as Map<String, dynamic>; | ||
| final subjectTokenFieldName = | ||
| format['subject_token_field_name'] as String; | ||
| token = json[subjectTokenFieldName] as String; | ||
| } | ||
| return token; | ||
| } | ||
| throw UnsupportedError( | ||
| 'Unsupported credential source type. Must provide file or url.', | ||
| ); | ||
| } |
There was a problem hiding this comment.
The _getSubjectToken method uses unsafe casts (as) which can lead to runtime errors if the credential_source map has an unexpected structure. It would be more robust to use pattern matching to safely access and cast values from the map. Additionally, throwing a generic Exception for HTTP failures is not ideal; a more specific exception like ServerRequestFailedException would be more consistent with the rest of the codebase and provide better error handling for consumers.
Future<String> _getSubjectToken() async {
final source = _credentialSource;
if (source case {'file': final String file}) {
return await File(file).readAsString();
} else if (source case {'url': final String url}) {
final headers = switch (source['headers']) {
final Map<String, dynamic> h =>
h.map((key, value) => MapEntry(key, value.toString())),
_ => null,
};
final response = await baseClient.get(
Uri.parse(url),
headers: headers,
);
if (response.statusCode != 200) {
throw ServerRequestFailedException(
'Failed to retrieve subject token from URL: $url. '
'Status code: ${response.statusCode}',
responseContent: response.body,
statusCode: response.statusCode,
);
}
var token = response.body;
if (source['format']
case {
'type': 'json',
'subject_token_field_name': final String fieldName
}) {
final json = jsonDecode(token) as Map<String, dynamic>;
if (json[fieldName] case final String subjectToken) {
token = subjectToken;
} else {
throw ArgumentError(
'Subject token field "$fieldName" not found in JSON response.',
);
}
}
return token;
}
throw UnsupportedError(
'Unsupported credential source type. Must provide file or url.',
);
}
kevmoo
left a comment
kevmoo
left a comment
There was a problem hiding this comment.
Addressed the feedback regarding pattern matching in _getSubjectToken and exception types. Thanks for the suggestions!
1 participant
Features:
external_accounttype in Application Default Credentials (ADC).StsAuthClientandclientViaStsTokenExchangeto handle token exchange with Google SecurityToken Service.Tests:
sts_auth_client_test.dartto verify token exchange functionality.adc_test.dartto verifyexternal_accountflow.